lib/commit: Try checksum+hardlink for untrusted local same-uid repos

This mainly helps flatpak for enabling a hardlink-able local pull
during deploy in the --system case.  We assume the files are immutable
when owned by the same uid.

See https://github.com/ostreedev/ostree/issues/1723
and https://github.com/flatpak/flatpak/pull/2342

Closes: #1776
Approved by: uajain

diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c
index f8f8d07021089abaad46ea3cd0f40604b555ee47..6cd3fcf470fd434514602f08cde7a913d8f9e9b9 100644 (file)
--- a/src/libostree/ostree-repo-commit.c
+++ b/src/libostree/ostree-repo-commit.c
@@ -4101,12 +4101,18 @@ import_is_bareuser_only_conversion (OstreeRepo *src_repo,
     && objtype == OSTREE_OBJECT_TYPE_FILE;
 }
 
-/* Returns TRUE if we can potentially just call link() to copy an object. */
+/* Returns TRUE if we can potentially just call link() to copy an object;
+ * if untrusted the repos must be owned by the same uid.
+ */
 static gboolean
 import_via_reflink_is_possible (OstreeRepo *src_repo,
                                 OstreeRepo *dest_repo,
-                                OstreeObjectType objtype)
+                                OstreeObjectType objtype,
+                                gboolean    trusted)
 {
+  /* Untrusted pulls require matching ownership */
+  if (!trusted && (src_repo->owner_uid != dest_repo->owner_uid))
+    return FALSE;
   /* Equal modes are always compatible, and metadata
    * is identical between all modes.
    */
@@ -4167,13 +4173,6 @@ import_one_object_direct (OstreeRepo    *dest_repo,
   char loose_path_buf[_OSTREE_LOOSE_PATH_MAX];
   _ostree_loose_path (loose_path_buf, checksum, objtype, dest_repo->mode);
 
-  if (!import_via_reflink_is_possible (src_repo, dest_repo, objtype))
-    {
-      /* If we can't reflink, nothing to do here */
-      *out_was_supported = FALSE;
-      return TRUE;
-    }
-
   /* hardlinks require the owner to match and to be on the same device */
   const gboolean can_hardlink =
     src_repo->owner_uid == dest_repo->owner_uid &&
@@ -4339,7 +4338,7 @@ _ostree_repo_import_object (OstreeRepo           *self,
    */
   const gboolean is_bareuseronly_conversion =
     import_is_bareuser_only_conversion (source, self, objtype);
-  gboolean try_direct = trusted;
+  gboolean try_direct = TRUE;
 
   /* If we need to do bareuseronly verification, or we're potentially doing a
    * bareuseronly conversion, let's verify those first so we don't complicate
@@ -4379,11 +4378,20 @@ _ostree_repo_import_object (OstreeRepo           *self,
         }
     }
 
-   /* We try to import via reflink/hardlink. If the remote is explicitly not trusted
-   * (i.e.) their checksums may be incorrect, we skip that.
-   */
-  if (try_direct)
+  /* First, let's see if we can import via reflink/hardlink. */
+  if (try_direct && import_via_reflink_is_possible (source, self, objtype, trusted))
     {
+      /* For local repositories, if the untrusted flag is set, we verify the
+       * checksum first. This assumes then that the files are immutable - the
+       * above check verified that the owner uids match.
+       */
+      if (!trusted)
+        {
+          if (!ostree_repo_fsck_object (source, objtype, checksum,
+                                        cancellable, error))
+            return FALSE;
+        }
+
       gboolean direct_was_supported = FALSE;
       if (!import_one_object_direct (self, source, checksum, objtype,
                                      &direct_was_supported,

diff --git a/tests/basic-test.sh b/tests/basic-test.sh
index d112533ee6c312c9710af02678cdefc1d1449684..0c80a5f52e925be5a2806691112686ff4192a346 100644 (file)
--- a/tests/basic-test.sh
+++ b/tests/basic-test.sh
@@ -21,7 +21,7 @@
 
 set -euo pipefail
 
-echo "1..$((86 + ${extra_basic_tests:-0}))"
+echo "1..$((88 + ${extra_basic_tests:-0}))"
 
 CHECKOUT_U_ARG=""
 CHECKOUT_H_ARGS="-H"
@@ -364,6 +364,39 @@ if ! skip_one_without_user_xattrs; then
     echo "ok pull-local --bareuseronly-files"
 fi
 
+rm repo2 -rf
+ostree_repo_init repo2 --mode="$mode"
+$CMD_PREFIX ostree --repo=repo2 pull-local --untrusted repo test2
+target_file_object=$(ostree_file_path_to_relative_object_path repo test2 baz/saucer)
+target_file_checksum=$(ostree_file_path_to_checksum repo test2 baz/saucer)
+assert_files_hardlinked repo{,2}/${target_file_object}
+echo "ok pull-local hardlinking, untrusted"
+
+if grep -q 'mode=bare' repo/config; then
+    # Now copy/corrupt an object in a 3rd repo, pull into 2nd (leaving the first pristine)
+    rm repo{2,3} -rf
+    ostree_repo_init repo2 --mode="$mode"
+    ostree_repo_init repo3 --mode="$mode"
+    # Pull into 3rd repo, corrupt an object
+    $CMD_PREFIX ostree --repo=repo3 pull-local repo test2
+    cp -a --reflink=auto repo3/${target_file_object}{,.tmp}
+    mv repo3/${target_file_object}{.tmp,}
+    echo blah >> repo3/${target_file_object}
+    if $CMD_PREFIX ostree --repo=repo2 pull-local --untrusted repo3 2>err.txt; then
+        assert_not_reached "pulled --untrusted from corrupted repo"
+    fi
+    assert_file_has_content err.txt 'Corrupted.*'${target_file_checksum}
+    rm -f err.txt
+    # But this one should succeed
+    $CMD_PREFIX ostree --repo=repo2 pull-local repo3
+    if $CMD_PREFIX ostree --repo=repo2 fsck 2>err.txt; then
+        fatal "repo should have pulled corrupted object"
+    fi
+    assert_file_has_content err.txt 'Corrupted.*'${target_file_checksum}
+fi
+echo "ok pull-local --untrusted corruption"
+rm repo{2,3} -rf
+
 # This is mostly a copy of the suid test in test-basic-user-only.sh,
 # but for the `pull --bareuseronly-files` case.
 cd ${test_tmpdir}
@@ -385,7 +418,7 @@ echo "ok pull-local (bareuseronly files)"
 
 if ! skip_one_without_user_xattrs; then
     cd ${test_tmpdir}
-    ${CMD_PREFIX} ostree --repo=repo2 checkout ${CHECKOUT_U_ARG} test2 test2-checkout-from-local-clone
+    ${CMD_PREFIX} ostree --repo=repo checkout ${CHECKOUT_U_ARG} test2 test2-checkout-from-local-clone
     cd test2-checkout-from-local-clone
     assert_file_has_content yet/another/tree/green 'leaf'
     echo "ok local clone checkout"

diff --git a/tests/test-basic-user-only.sh b/tests/test-basic-user-only.sh
index 5f27014ecf0636d4410b22188305ac8c0bab2736..f65094fd5eead5c33b77117da7aa37f0a5e0fd58 100755 (executable)
--- a/tests/test-basic-user-only.sh
+++ b/tests/test-basic-user-only.sh
@@ -23,7 +23,8 @@ set -euo pipefail
 
 . $(dirname $0)/libtest.sh
 
-setup_test_repository "bare-user-only"
+mode="bare-user-only"
+setup_test_repository "$mode"
 extra_basic_tests=5
 . $(dirname $0)/basic-test.sh
 

diff --git a/tests/test-basic-user.sh b/tests/test-basic-user.sh
index 7bdb6a0ce232c8a7255b597f941c62255b7b52a5..e56f828aa78914cd60e1b68d450e7a9f2d7ec108 100755 (executable)
--- a/tests/test-basic-user.sh
+++ b/tests/test-basic-user.sh
@@ -25,7 +25,8 @@ set -euo pipefail
 
 skip_without_user_xattrs
 
-setup_test_repository "bare-user"
+mode="bare-user"
+setup_test_repository "$mode"
 
 extra_basic_tests=6
 . $(dirname $0)/basic-test.sh

diff --git a/tests/test-basic.sh b/tests/test-basic.sh
index c7f28c8c57e13fdadd24850a1e432257c796beb8..b1bc37e571e1745e6f7e0089c9c34da08b5b8eb0 100755 (executable)
--- a/tests/test-basic.sh
+++ b/tests/test-basic.sh
@@ -25,5 +25,6 @@ set -euo pipefail
 
 skip_without_no_selinux_or_relabel
 
-setup_test_repository "bare"
+mode="bare"
+setup_test_repository "$mode"
 . $(dirname $0)/basic-test.sh

diff --git a/tests/test-pull-untrusted.sh b/tests/test-pull-untrusted.sh
index 4c972599445020dbc23e42c7e214c9577e2ed385..ec48eb3eb4823af3ef7adcc3776bb1f949ef08b6 100755 (executable)
--- a/tests/test-pull-untrusted.sh
+++ b/tests/test-pull-untrusted.sh
@@ -25,52 +25,10 @@ set -euo pipefail
 
 . $(dirname $0)/libtest.sh
 
-echo '1..4'
+echo '1..1'
 
 setup_test_repository "bare"
 
-cd ${test_tmpdir}
-mkdir repo2
-ostree_repo_init repo2 --mode="bare"
-
-${CMD_PREFIX} ostree --repo=repo2 --untrusted pull-local repo
-
-find repo2 -type f -links +1 | while read line; do
-    assert_not_reached "pull-local created hardlinks"
-done
-echo "ok pull-local --untrusted didn't hardlink"
-
-# Corrupt repo
-for i in ${test_tmpdir}/repo/objects/*/*.file; do
-
-    # make sure it's not a symlink
-    if [ -L $i ]; then
-        continue
-    fi
-
-    echo "corrupting $i"
-    echo "broke" >> $i
-    break;
-done
-
-rm -rf repo2
-mkdir repo2
-ostree_repo_init repo2 --mode="bare"
-if ${CMD_PREFIX} ostree --repo=repo2 pull-local repo; then
-    echo "ok trusted pull with corruption succeeded"
-else
-    assert_not_reached "corrupted trusted pull unexpectedly succeeded!"
-fi
-
-rm -rf repo2
-ostree_repo_init repo2 --mode="bare"
-if ${CMD_PREFIX} ostree --repo=repo2 pull-local --untrusted repo; then
-    assert_not_reached "corrupted untrusted pull unexpectedly failed!"
-else
-    echo "ok untrusted pull with corruption failed"
-fi
-
-
 cd ${test_tmpdir}
 tar xf ${test_srcdir}/ostree-path-traverse.tar.gz
 rm -rf repo2